-
Notifications
You must be signed in to change notification settings - Fork 24
chore: move db models to wire data package (second attempt) - WPB-19688 #3819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test Results 7 files 654 suites 7m 48s ⏱️ For more details on these failures, see this check. Results for commit 669dfc7. ♻️ This comment has been updated with latest results. |
…reReturned flakiness
WireData/Sources/WireData/Schema/zmessaging.xcdatamodeld/zmessaging2.131.0.xcdatamodel/contents
Show resolved
Hide resolved
netbe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments, but otherwise looks good to me :)
Question: Can we close #3521 ?
wire-ios-data-model/Resources/zmessaging.xcdatamodeld/zmessaging2.110.0.xcdatamodel/contents
Show resolved
Hide resolved
# Conflicts: # wire-ios-data-model/WireDataModel.xcodeproj/project.pbxproj
Issue
This PR is a new version of #3521. As that PR was quite far behind
developand the core data model has changed, I decided to combine changes from the original PR with a few updates.This PR moves the
zmessagingandZMEventModeldatabase models fromWireDataModelframework toWireDatapackage ensuring that the managed objects that were previously pointing to the Current Product Module now points to WireDataModel (where still reside their related subclasses).To achieve this:
WireDataWireDataModel- these can be moved one by one as needed.is previously it was set asCurrent Product Module` - see notes below.WireRequestStrategyTestHostwas updated to link toWireRequestStrategy. Without doing this theWireDataBundle.bundlewould crash trying to find the bundle. This change ensures that the compiled core data module resides in the test host and is therefore accessible in tests. This was the main difference to the original PR: chore: move db models to wire data package - WPB-19688 #3521WireDomainto address the above issue as it was missing a test host.This PR is largely @jullianms good work - my changes mainly relate to resolving test host issue
Some additional notes
Some things I've learnt which I didn't know before regarding the classes module:
This can have 3 states:
Global Namespaceis required for managed object subclasses that are obj-c or those that are used from obj-c. This is because obj-c uses a global namespace (hence the need to add prefixes).Current Product Moduleis likeCustombut automatically sets the target based on where the database models reside. CurrentlyCurrent Product Moduleresolves toWireDataModelbut with this PR we move the database models toWireDatawhile keeping the classes inWireDataModel. This means that we have to updates settings ofCurrent Product ModuletoWireDataModel.I was surprised at the need to update each version of
zmessaging.xcdatamodeld, not just2.129but2.128,2.127, etc. This was necessary because some of our migration tests depend on our managed object subclasses with older database versions. For example,DatabaseMigrationTests_Conversations.testThatItPerformsMigrationFrom106_deleteConversationCascadesToParticipantRole(). I think this is a little problematic as there is no reason why the current version ofParticipantRoleis compatible with version2.106.0for example. It would be better to use just NSManagedObject with KVC. However it's probably best to leave things how they are.Testing
Checklist
[WPB-XXX].